gh-156780: Emscripten: add missing EM_JS_DEPS - #156798
Conversation
|
@hoodmane first PR that add the EM_JS_DEPS() everywhere |
259acbf to
692abd8
Compare
692abd8 to
f770de5
Compare
|
Noting that CI is currently failing because of a duplicated symbol; that seems to suggest there's a need to collate these declarations into a single coherent location so that we can avoid duplication, rather than co-locating them with usage. Also - can I ask that you please don't force push. We don't care about a "messy" commit history; CPython uses merge commits, so any messy history isn't preserved after merge. Force pushes also mean that we can lose the context for review comments, and it's really easy to force push a merge comment that pings the entire core team for comment. |
|
Emscripten's normal style guide is to put these next to the symbol that uses them so I think that's alright. I think we have a conflict with #156335, need to merge a main that includes that commit and fix the duplicate symbol. |
EM_JS bodies are emitted verbatim, so Emscripten cannot see the JS symbols they reference. Declare them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
8015a97 to
18a9541
Compare
|
@freakboy3742 "Noting that CI is currently failing because of a duplicated symbol" ? The build fail no? The issue here is that my fork was not up to date with main. Also about the force push well noted, I had to do it here to properly rebase on latest master |
Split out of #156781, as suggested by @hoodmane.
EM_JSbodies are emitted verbatim, so Emscripten's dependency tracker cannot see the JS symbols they reference and silently omits them.Declared here:
$FS,$PATH,$FS_getMode,$resolveGlobalSymbolandemscripten_exit_with_live_runtimeinemscripten_syscalls.c, and$wasmTable,$wasmMemory,$addFunction,$addOnPreRuninemscripten_trampoline.c.No change for
MAIN_MODULEbuilds. Without it, the link now fails withinstead of a
ReferenceErrorduringinitRuntime.